fix(config): preserve production state paths#3864
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| ): Effect.fn.Return<ServerDerivedPaths, never, Path.Path> { | ||
| const { join } = yield* Path.Path; | ||
| const stateDir = join(baseDir, devUrl !== undefined ? "dev" : "userdata-v2"); | ||
| const stateDir = join(baseDir, devUrl !== undefined ? "dev" : "userdata"); |
There was a problem hiding this comment.
🟡 Medium src/config.ts:96
Changing stateDir from "userdata-v2" back to "userdata" makes the server look for production state in join(baseDir, "userdata") instead of join(baseDir, "userdata-v2"), where the current base revision writes it. There is no migration or fallback lookup, so after upgrading the server creates a fresh empty state.sqlite, settings, logs, and environment IDs under userdata, causing existing user history and configuration to appear lost. If this revert is intentional, consider adding a migration or fallback so existing userdata-v2 state is preserved.
Also found in 1 other location(s)
apps/desktop/src/app/DesktopEnvironment.ts:158
Reverting
stateDirtopath.join(baseDir, "userdata")strands the desktop's existing production files from the base revision, which wrotedesktop-settings.json,client-settings.json, andsaved-environments.jsonunder.../userdata-v2. The upgraded app now reads and writes those files in.../userdatainstead, so users coming from the current base build lose their saved environments and desktop settings until they manually copy the old directory.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/config.ts around line 96:
Changing `stateDir` from `"userdata-v2"` back to `"userdata"` makes the server look for production state in `join(baseDir, "userdata")` instead of `join(baseDir, "userdata-v2")`, where the current base revision writes it. There is no migration or fallback lookup, so after upgrading the server creates a fresh empty `state.sqlite`, settings, logs, and environment IDs under `userdata`, causing existing user history and configuration to appear lost. If this revert is intentional, consider adding a migration or fallback so existing `userdata-v2` state is preserved.
Also found in 1 other location(s):
- apps/desktop/src/app/DesktopEnvironment.ts:158 -- Reverting `stateDir` to `path.join(baseDir, "userdata")` strands the desktop's existing production files from the base revision, which wrote `desktop-settings.json`, `client-settings.json`, and `saved-environments.json` under `.../userdata-v2`. The upgraded app now reads and writes those files in `.../userdata` instead, so users coming from the current base build lose their saved environments and desktop settings until they manually copy the old directory.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Changes production state directory paths from 'userdata-v2' to 'userdata' without apparent migration logic. An unresolved review comment identifies this could orphan existing user data, causing settings and state to appear lost after upgrade. You can customize Macroscope's approvability policy. Learn more. |
Summary
userdatadirectorydevand align SSH runtime discoveryOpen question
#2829 deliberately introduced
userdata-v2in3b864044c. Reverting that rename preserves installations that only have the originaluserdatastate, but it can orphan state already written by v2 preview builds. This PR is held as a draft while upstream decides between reverting before release or retaining v2 with an explicit migration/import policy.As v2 was never released, I'm leaning towards migrating v1 userdata to v2 in place with an explicit backup policy. This would break downgrades potentially--and I suppose that's exactly why the rename was introduced.
Validation
vp checkvp run typecheckAddresses review feedback on #2829.